Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ci): build each crate individually #4640

Merged
merged 17 commits into from
Jun 21, 2022
Merged

feat(ci): build each crate individually #4640

merged 17 commits into from
Jun 21, 2022

Conversation

gustavovalverde
Copy link
Member

@gustavovalverde gustavovalverde commented Jun 17, 2022

Motivation

Sometimes we add code that needs a feature to a Zebra crate, then activate that feature in zebrad. But we forget to activate the feature in the crate that actually contains the code.

For an example, see PR #1365, where individual crates use features in their dependencies, but don't declare those features in their Cargo.toml.

Fixes #1364
Fixes #4550

Designs

  • Build each Zebra crate individually in a CI job, including Zebra's custom Tower crates
  • Fail the PR if any build fails.

Solution

  • Create a specific workflow to accomplish this task
  • Use a matrix strategy to have a separate job for each crate, with individual logs
  • Keep a fail-fast strategy, as we don't want to keep consuming minutes if a crate build failed

Review

Not ready

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behaviour
  • Tests for Errors

Follow Up Work

It would be better to generate the crate names automatically, but this task would need further analysis to be able to generate a file with the needed GHA format for a matrix.

@gustavovalverde gustavovalverde added A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement P-High 🔥 I-build-fail Zebra fails to build labels Jun 17, 2022
@gustavovalverde gustavovalverde requested a review from a team as a code owner June 17, 2022 21:28
@gustavovalverde gustavovalverde self-assigned this Jun 17, 2022
@gustavovalverde gustavovalverde requested review from conradoplg and removed request for a team June 17, 2022 21:28
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some things I noticed, happy to leave someone else to do the full review.

Are we going to make all these jobs required in CI?
That seems like a lot of jobs to add, we might want to think about making a final job that depends on all of them.

.github/workflows/build-crates-individually.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thanks for this!

@teor2345
Copy link
Contributor

I have this list of tasks ready to go in the main branch protection rules, as soon as this PR merges.

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add patch jobs to this PR?

We need them if we want to require these jobs in CI.

@gustavovalverde
Copy link
Member Author

patch file added

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can delete some redundant config lines, but it doesn't really matter either way

.github/workflows/build-crates-individually.patch.yml Outdated Show resolved Hide resolved
.github/workflows/build-crates-individually.patch.yml Outdated Show resolved Hide resolved
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, let's merge and see how we go

mergify bot added a commit that referenced this pull request Jun 21, 2022
@mergify mergify bot merged commit b6a59a9 into main Jun 21, 2022
@mergify mergify bot deleted the feat-build-crates branch June 21, 2022 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement I-build-fail Zebra fails to build
Projects
None yet
2 participants